Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement reload / go to next level while demo playing #1107

Merged
merged 5 commits into from
Nov 7, 2023
Merged

Conversation

JNechaevsky
Copy link
Collaborator

Initial PR: JNechaevsky/international-doom#27

Remaining TODO: no luck with proper music changing in Crispy code. Needs some investigation why, and probably will need some help here.

@fabiangreffrath
Copy link
Owner

We already have a demowarp feature. Could you try to use this instead?

Have to say, that condition in S_Start was well hidden!
@JNechaevsky
Copy link
Collaborator Author

We already have a demowarp feature. Could you try to use this instead?

Means, instead of using G_DemoGotoNextLevel, use crispy->demowarp in key_nextlevel handling?

@fabiangreffrath
Copy link
Owner

That was my initial idea, yes. But this won't work because we cannot know what the next level will be (player may use regular or secret exit). Just scrap my idea.

@JNechaevsky
Copy link
Collaborator Author

Ah, got it. Yeah, we just running tics here as fast as possible, not indicating "go to this/that map" and stopping "full speed" right before player spawning in P_SpawnPlayer.

Guess, it's your review-turn now, I have nothing to add technically, except few thoughts:

  • Keys are working in demo loop. Could be useful for testing, or... Or is it a bad idea?
  • No smooth demo bar while jumping. Which is good, because this making jumping as fast as it gets. And hey, demo bar is not drawing on top of wipes.

@JNechaevsky JNechaevsky marked this pull request as ready for review October 30, 2023 13:15
@JNechaevsky
Copy link
Collaborator Author

Small problem, not exactly related to this PR: framerate is forcefully uncapped after using -warp in demo playing, but crispy->vsync remains untouched.

@JNechaevsky
Copy link
Collaborator Author

To fix framerate capping issue, we have to extend this condition:

// Turn on vsync if we aren't in a -timedemo
if ((!singletics && mode.refresh_rate > 0) || demowarp)
                                                ^^^^

Since demo warp is using singletics, which, in return, preventing vsync flag to be accounted. But this way demowarp must be externalized to upper level code, and the question is - where exactlly? Should it be i_video.h or extern int demowarp right in SetVideoMode?

@fabiangreffrath
Copy link
Owner

I don't think this will link with the non-Doom games if you introduce demowarp as only an extern in the i_video.c context. You'll wither need a demowarp instantiation in each game, or you instantiate it in i_video.c itself.

@JNechaevsky
Copy link
Collaborator Author

Pardon me, it's far simpler as demowarp is a part of crispy_t structure. This will fix capping issue without any externals:

    // Turn on vsync if we aren't in a -timedemo
    if ((!singletics && mode.refresh_rate > 0) || crispy->demowarp)
                                                  ^^^^^^^^^^^^^^^^

We can't move it to condition below since !singletics is doing it's job.

@fabiangreffrath
Copy link
Owner

Well then.

@turol
Copy link
Collaborator

turol commented Oct 31, 2023

Please put extern in a header where it's also visible to the actual declaration. This helps the compiler catch bugs. The check-extern.sh script checks this. It's enabled in CI for Chocolate but not for Crispy.

@JNechaevsky
Copy link
Collaborator Author

@turol, understood for future, thanks.

JNechaevsky referenced this pull request in JNechaevsky/international-doom Oct 31, 2023
Just in case. Otherwise, nothing more than black screen will appear after pressing "Restart level/demo". Going to next level is working fine.
@JNechaevsky
Copy link
Collaborator Author

Thanks for approval! Can we merge it now? On next PR I would like to implement IDCLEV'ing to any levels, not just next ones while demo playback, it's fairly simple (JNechaevsky/international-doom@faeb5a9).

For almost a week I was wondering what else can be done regarding demo playback, but no, nothing, zero ideas. Looks likes current features covers all possible needs, at least for simple wathching.

So far, info for changelog. Please correct my wording if needed:

  • If vsync is active, framerate no longer forcefully uncapped after using demo warp (i.e. -playdemo demo.lmp -warp xx)
  • Improved Crispy shortcut keys behavior while demo playback:
    • Restart level/demo: now restart demo but keeps it playing.
    • Go to next level: now warping to next level and keeps demo playing.

@fabiangreffrath
Copy link
Owner

Thanks for approval! Can we merge it now?

That's what "aprroval" means, right? 😉

@JNechaevsky
Copy link
Collaborator Author

That's what "aprroval" means, right? 😉

Nothing can do with myself, I have a huge fondness for subordination when changing code in Crispy. ☺️

@JNechaevsky JNechaevsky merged commit f6c67d1 into master Nov 7, 2023
6 checks passed
@JNechaevsky JNechaevsky deleted the demojumps branch November 7, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants